Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Implement Grid.open_file method support non xarray formats including Shapefile/Geojson #808

Merged
merged 65 commits into from
Sep 19, 2024
Merged

Implement Grid.open_file method support non xarray formats including Shapefile/Geojson #808

merged 65 commits into from
Sep 19, 2024

Conversation

rajeeja
Copy link
Contributor

@rajeeja rajeeja commented Jun 3, 2024 •
edited by philipc2
Loading

Closes #635

Support Polygon and Multipolygon

@rajeeja rajeeja requested a review from philipc2 June 3, 2024 18:55
rajeeja added 15 commits June 3, 2024 14:01
@rajeeja
o Add geopandas dependency
fb10484
@rajeeja
o Fix tests, TODO: Fix API and make open_grid work.
fa3b73f
@rajeeja
Possible that np.intp could have different ranges on 32-bit versus 64…
a503c51 
…-bit systems. Using np.int32 for padding, this might fix the overflow error on windows machine: https://github.com/UXARRAY/uxarray/actions/runs/9357045842/job/25755876561\?pr\=808\#step:8:87
@rajeeja
trying np.int16
1353d2b
@rajeeja
Merge branch 'main' into rajeeja/io_shapefile
66ccc8b
@rajeeja
Merge branch 'rajeeja/io_shapefile' of github.com:UXARRAY/uxarray int…
a7b8328 
…o rajeeja/io_shapefile
@rajeeja
o Make a small example, fix bug with max size for connectivity arrray…
9664430 
…, this should pass for windows now as the example is much smaller
@rajeeja
o Fix setting the datatype for empty connectivity array to a np.int16…
1f12543 
…, initial thought was to optimize, but this led to issues with cross platform handling in integer types, specially windows, hopefully this might fix it. As in previous commit, it was not an issue of large mesh size that was causing this.
@rajeeja
o Setting the dtype to int32 as leaving empty defaults to float
b5fe8ed
@rajeeja
o Use np.intp for connectivity init
65901dd
@rajeeja
Merge branch 'main' into rajeeja/io_shapefile
0ec8002
@rajeeja
Merge branch 'main' into rajeeja/io_shapefile
00fc5fa
@rajeeja
o Check to see if np.fulll instead of pad works
07cfae7
@rajeeja
o Add test for multipolygon case and also test a file
c05cfda
@rajeeja
o Fix comments
5ac3e08
erogluorhan
erogluorhan approved these changes Sep 10, 2024
View reviewed changes
Copy link
Member

@erogluorhan erogluorhan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great addition; thanks a lot!

@erogluorhan
Copy link
Member

We should consider a basic User Guide notebook for this too. I can provide the quad-hexagon grid in multiple different formats (i.e. shape, geoparquet, etc.) to showcase this functionality.

I have some mesh files as a part of this, we want to be careful with how many we bundle as a part of the repo.

Maybe using online files rather than having them all hosted in the repo?

@rajeeja rajeeja mentioned this pull request Sep 10, 2024
Open
philipc2
philipc2 requested changes Sep 11, 2024
View reviewed changes
Copy link
Member

@philipc2 philipc2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update the title & linked issue to reflect the generalization changes that were made.

@philipc2
Copy link
Member

We should consider a basic User Guide notebook for this too. I can provide the quad-hexagon grid in multiple different formats (i.e. shape, geoparquet, etc.) to showcase this functionality.

I have some mesh files as a part of this, we want to be careful with how many we bundle as a part of the repo.

Maybe using online files rather than having them all hosted in the repo?

Do we have an issue open for this topic in general?

philipc2
philipc2 requested changes Sep 11, 2024
View reviewed changes
docs/internal_api/index.rst Outdated Show resolved Hide resolved
uxarray/core/api.py Outdated Show resolved Hide resolved
uxarray/grid/grid.py Outdated Show resolved Hide resolved
@rajeeja rajeeja requested a review from philipc2 September 12, 2024 23:36
philipc2
philipc2 requested changes Sep 14, 2024
View reviewed changes
Copy link
Member

@philipc2 philipc2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few comments. Please update the PR title & linked issue as well.

uxarray/grid/grid.py Outdated Show resolved Hide resolved
uxarray/grid/grid.py Outdated Show resolved Hide resolved
uxarray/grid/grid.py Outdated Show resolved Hide resolved
@rajeeja
Copy link
Contributor Author

rajeeja commented Sep 14, 2024

We should consider a basic User Guide notebook for this too. I can provide the quad-hexagon grid in multiple different formats (i.e. shape, geoparquet, etc.) to showcase this functionality.

I have some mesh files as a part of this, we want to be careful with how many we bundle as a part of the repo.

Maybe using online files rather than having them all hosted in the repo?

Do we have an issue open for this topic in general?

#946 would cover this also, not a explicit issue for this.

@rajeeja rajeeja requested a review from philipc2 September 18, 2024 16:28
@philipc2 philipc2 changed the title Shapefile Reader Implement Grid.open_file method support non xarray formats including Shapefile/Geojson Sep 18, 2024
philipc2
philipc2 approved these changes Sep 19, 2024
View reviewed changes
Copy link
Member

@philipc2 philipc2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Other than the one comment, feel free to merge it once it's been resolved.

uxarray/core/api.py Outdated Show resolved Hide resolved
rajeeja and others added 3 commits September 19, 2024 09:20
@rajeeja rajeeja force-pushed the rajeeja/io_shapefile branch from aebfc26 to 610e029 Compare September 19, 2024 15:27
@rajeeja rajeeja merged commit 6862950 into main Sep 19, 2024
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@philipc2 philipc2 philipc2 approved these changes

@erogluorhan erogluorhan erogluorhan approved these changes

Assignees

@rajeeja rajeeja

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Implement open_file method of Grid class, support non xarray formats Shapefile/Geojson
3 participants
@rajeeja @philipc2 @erogluorhan